Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

always allow decoding of status when returned from the API #35810

Merged
merged 1 commit into from
Nov 3, 2016
Merged

always allow decoding of status when returned from the API #35810

merged 1 commit into from
Nov 3, 2016

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Oct 28, 2016 •
edited by k8s-oncall

unversioned.Status should be able to come back from any API version and still be properly decoded. This doesn't happen today by default.

@smarterclayton Our projectrequest endpoint returns a Status object on a 200 return from list to indicate everything went well. This (or something like it) is needed to make the API accepted by kubectl. Alternatively, we change the API to return a different (still not a Project) value from list, which still feels wrong.

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Oct 28, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit ba46a0bfa907604b232a3595884ecfcccc9d0d6e. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@smarterclayton
Copy link
Contributor

Test

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 31, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 29171ec171065ca86a3d878ce8b7f24d9cb06fb5. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@smarterclayton
Copy link
Contributor

Add a test please.

@deads2k
Copy link
Contributor Author

deads2k commented Oct 31, 2016

Add a test please.

Without a cut-down API server done in an openshift test-integration.sh style, I'm not sure I can. The kube test infrastructure just doesn't have the capability.

@smarterclayton
Copy link
Contributor

You can test rest client against a semi real server in apiserver_test.go, which has an almost real server

@deads2k
Copy link
Contributor Author

deads2k commented Nov 1, 2016

unit test added.

@deads2k deads2k added release-note-none Denotes a PR that doesn't merit a release note. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Nov 1, 2016
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 1, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit 5563fad162e29ce929a7cb873d38bc7e66d2fb70. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@deads2k deads2k added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Nov 1, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit 8115d18eb16670e42202f3a049b61f0aafc2137f. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@deads2k deads2k added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Nov 1, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 235678d. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit 235678d. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 2, 2016 •
edited

@k8s-bot unit test this

flake already fixed in head.

@deads2k deads2k added this to the v1.5 milestone Nov 3, 2016
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 7c39115 into kubernetes:master Nov 3, 2016
@deads2k deads2k deleted the client-18-decode-status branch February 1, 2017 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@deads2k @k8s-ci-robot @smarterclayton @k8s-github-robot @googlebot